Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate unnecessary unescaping of tags #397

Closed
wants to merge 1 commit into from

Conversation

kornelski
Copy link

Fixes #341

@kornelski kornelski mentioned this pull request Jun 19, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #397 (8ba26d4) into master (e701c4d) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
+ Coverage   61.37%   61.44%   +0.06%     
==========================================
  Files          20       20              
  Lines       10157    10157              
==========================================
+ Hits         6234     6241       +7     
+ Misses       3923     3916       -7     
Flag Coverage Δ
unittests 61.44% <ø> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/events/mod.rs 73.96% <ø> (ø)
src/lib.rs 26.00% <0.00%> (ø)
src/reader.rs 87.51% <0.00%> (+0.09%) ⬆️
src/de/mod.rs 75.73% <0.00%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e701c4d...8ba26d4. Read the comment docs.

@Mingun
Copy link
Collaborator

Mingun commented Jun 19, 2022

I think, we need to remove that function entirely if is does the wrong thing. I'll hold this PR for a while, because I also prepare a PR that touches many of escaping/decoding functions and I need time to review this and find an initial PR that introduces it to understand what problem it solves.

Anyway, you feel free to change this PR to remove method and update a Changelog.md. Could you also use #118 instead of #341 in messages and in the changelog?

@kornelski
Copy link
Author

I've hidden these functions from docs, so that they're almost as good as deleted, but don't need semver-major bump. You could delete them next time you release a major version.

@Mingun
Copy link
Collaborator

Mingun commented Jun 20, 2022

We're anyway at 0.x stage so any release is breaking and I do not limit myself in using this opportunity. Actually, next 0.24.0 release already contains breaking changes, so one more is not a problem. In any case, we should remove any usage of deprecated items from our codebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

what's BytesStart::unescaped for?
3 participants